Explain how to work with subtree #70654

oli-obk · 2020-04-01T12:35:59Z

This PR just contains the usage documentation, we'll do actual moves in later PRs

Centril · 2020-04-01T12:39:49Z

Waiting with the review and r+ until the conversation re. "should we do this" that you're having with @eddyb has resolved itself. =P

Others should feel free to propose text tweaks though. :)

oli-obk · 2020-04-01T12:55:26Z

see #70655 for a live example of making clippy a subtree

nikomatsakis · 2020-04-01T13:09:54Z

A question that I didn't see answered: Who is expected to do the "git subtree push/pull"? Are we saying that this is the job of the (e.g.) clippy team, and they're going to do it on some kind of regular schedule?

CONTRIBUTING.md

RalfJung · 2020-04-01T13:41:40Z

s/subrepo/subtree/

I am confused, did we change the approach or did we just use the wrong term so far?

EDIT: there are just too many discussion going on too fast in concurrent threads right now. Looks like we indeed changed approaches but that was mentioned in one of the 3 other threads, not here, or so.^^

…ynchronizations

CONTRIBUTING.md

Centril · 2020-04-02T16:01:16Z

CONTRIBUTING.md

+You always need to specify the `-P` prefix to the subtree directory and the corresponding remote
+repository. If you specify the wrong directory or repository
+you'll get very fun merges that try to push the wrong directory to the wrong remote repository.
+Luckily you can just abort this without any consequences and try again. It is usually fairly obvious


How do you abort?

When pulling, git merge --abort I'd guess? We should test it.

Pushing should always succeed, but if you try to open PR from a malformed branch, it will just look very strange and be unmergeable.

Presumably you can redo it with the right directory specified and the PR will become correct (idk if it overwrites an existing branch though, or maybe it has a -f/--force flag like git push does?).

CONTRIBUTING.md

Centril · 2020-04-02T16:03:03Z

Reassigning to someone more git-savvy than me. :)

r? @Mark-Simulacrum

nikomatsakis · 2020-04-03T13:50:05Z

One final note: I think that this should also go into the rustc-dev-guide. I think it's fine to just land a chapter there that points to CONTRIBUTING.md as the canonical documentation point, though.

Mark-Simulacrum · 2020-04-04T20:08:15Z

CONTRIBUTING.md

+modified to move the files from the specified directory to the tool repo root.
+
+Make sure to not pick the `master` branch on the tool repo, so you can open a normal PR to the tool
+to merge that subrepo push.


It seems to me that the important thing is to not have git@github.com:rust-lang/rust-clippy.git as the repository URL, i.e., you should never be pushing into branches there. But which branch you choose in your fork is largely irrelevant (you can push to master if you like).

I don't have my own fork of clippy and miri, should I have?

I think it is better to keep personal feature branches out of the main repo. Not a strong opinion though.

@oli-obk Not to mention that you can only do this because of your powers on those repos, most contributors have to use a fork.

IMO for repos with many contributors the amount of feature branches in the main repo should be kept as low as possible. Clippy has even an issue, where a contributor complained about the amount of feature branches: rust-lang/rust-clippy#4745

Mark-Simulacrum · 2020-04-04T20:33:16Z

CONTRIBUTING.md

+you'll get very fun merges that try to push the wrong directory to the wrong remote repository.
+Luckily you can just abort this without any consequences by throwing away either the pulled commits
+in rustc or the pushed branch on the remote and try again. It is usually fairly obvious
+that this is happening because you suddenly get thousands of commits that want to be synchronized.


Am I correct that subtrees mean that we should no longer need to give particular priority to syncs from clippy upstream? i.e. they're "just normal PRs"? (They would essentially be no different to other PRs being made to that code).

Priority meaning @bors p=1? That definitely should end with the gating.

jup, all the "time critical" stuff happens on the tool's repo side and not when syncing into rustc. Unless there are bug fixes, but our policy for tool bugfixes and rustc bugfixes willl not have to differ.

Mark-Simulacrum · 2020-04-04T21:50:37Z

I want us to also answer these questions before we proceed here:

How should contributors behave with respect to subtrees? For example, if I want to contribute a new lint to clippy, must that happen on the clippy repository? Or can it also happen in rust-lang/rust?
- I would prefer for the answer to be "no, it must happen on the clippy repository" at least initially, I think, just for ease of review. This would mean that only changes which are related to rustc happen in this repository (e.g., renaming types which affects clippy). However this means that we do not make it as easy for existing rust-lang/rust contributors to work on clippy (they need to learn the clippy repository's build system and quirks). But I think that's a secondary goal and we should start off with "no new work" for clippy and other subtrees on rust-lang/rust.
How should tidy and other similar tooling behave? (e.g. should we enforce rustfmt on clippy)
- Obviously the ideal here is "yes" otherwise changes to files in src/tools/clippy likely have unrelated changes due to rustfmt-on-save and similar, but I can definitely see that being a hard position to accept given the relatively custom nature of rust-lang/rust's tidy. I think the best thing to do is for now not enforce tidy on the files, and continue ignoring the relevant directories in rustfmt.toml. This may change eventually, especially if we start using a non-nightly pinned rustfmt in rust-lang/rust.

flip1995 · 2020-04-05T15:12:21Z

must that happen on the clippy repository?

I think most new lints and features should still be implemented in the Clippy repository. If there was a new lint implemented in the rust repository, and this lint would fail the stricter CI of the Clippy repo, this lint would have to be re-reviewed and maybe modified during a sync.

Also the separation would be clearer: rustups in rust-lang/rust, everything else in the Clippy repo.

How should tidy and other similar tooling behave?

Enforcing rustfmt on Clippy would not be a problem, since we're already doing this. Maybe we would have to adapt our rustfmt.toml to match the one in rust-lang/rust and then reformat once. Does tidy enforce other things, that would concern Clippy, other than rustfmt?

Mark-Simulacrum · 2020-04-05T15:22:13Z

Tidy mostly only enforces rustfmt; we do currently pin to a specific nightly which might not be optimal for clippy -- but I think rustfmt at least claims stability (even if that sometimes doesn't hold up in practice, I think). I suspect we'd for now hold off on enforcing rustfmt for the clippy repository (we can land that at a future date), just to avoid bundling too much into the initial rollout. But I suspect we'd fairly quickly want to start doing so, especially as it sounds like at least in the clippy case that should be fairly easy.

flip1995 · 2020-04-05T15:32:20Z

We're currently looking to enhance our dev tooling to behave more like ./x.py rust-lang/rust-clippy#5394. We could also enforce the fixed nightly rustfmt there. But I agree to revisit this later.

RalfJung · 2020-04-05T15:39:20Z

Also the separation would be clearer: rustups in rust-lang/rust, everything else in the Clippy repo.

From a Miri perspective, I very much like this approach.

nikomatsakis · 2020-04-06T13:39:05Z

I agree with @Mark-Simulacrum's suggestions, although the "tidy" thing is definitely a drag.

Mark-Simulacrum · 2020-04-11T14:10:08Z

I would also echo @nikomatsakis' in saying that I would like to see a link in the rustc-dev-guide pointing at these docs, but that can happen once we land this PR, of course.

Okay, I think we have all of our checkboxes checked here. @oli-obk -- do you know of anything outstanding? If not, let's kick off FCP on the tracking issue (rust-lang/compiler-team#266).

Mark-Simulacrum · 2020-04-13T21:24:24Z

Per rust-lang/compiler-team#266 (comment), @bors r+ rollup

Once this lands I can take a look at the actual submodule to subtree PR.

bors · 2020-04-13T21:24:26Z

📌 Commit 7928c45 has been approved by Mark-Simulacrum

@ghost

Rollup of 4 pull requests Successful merges: - rust-lang#70654 (Explain how to work with subtree) - rust-lang#71092 (Remove some usage of `DUMMY_HIR_ID`) - rust-lang#71103 (Add test case for type aliasing `impl Sized`) - rust-lang#71109 (allow const generics in const fn) Failed merges: r? @ghost

@eddyb

…acrum Make clippy a git subtree instead of a git submodule r? @eddyb cc rust-lang#70651 documentation at rust-lang#70654

matthiaskrgr · 2020-05-04T12:01:30Z

CONTRIBUTING.md

+```
+git subtree push -P src/tools/clippy git@github.com:your-github-name/rust-clippy sync-from-rust
+```
+


This is not working at all on my machine:

git subtree push -P src/tools/clippy git@github.com:matthiaskrgr/rust-clippy2 sync-from-rust git push using: git@github.com:matthiaskrgr/rust-clippy2 sync-from-rust /usr/lib/git-core/git-subtree: line 757: 171266 Done eval "$grl" 171267 Segmentation fault (core dumped) | while read rev parents; do process_split_commit "$rev" "$parents" 0; done

O_o can you run a top in parallel and have a look at your memory consumption?

rust-highfive assigned Centril Apr 1, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2020

Explain how to work with subrepos

04c8191

oli-obk force-pushed the subrepo_docs branch from aa3c958 to 04c8191 Compare April 1, 2020 12:36

oli-obk changed the title ~~Explain how to work with subrepos~~ Explain how to work with subtree Apr 1, 2020

s/subrepo/subtree/

56d0c81

oli-obk mentioned this pull request Apr 1, 2020

Make clippy a git subtree instead of a git submodule #70655

Merged

pietroalbini reviewed Apr 1, 2020

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Git subtree is upstream, no need to install

521eb0d

RalfJung mentioned this pull request Apr 1, 2020

Replace submodules with subtrees #70651

Closed

oli-obk mentioned this pull request Apr 1, 2020

Use git subtree (instead of git submodule) for external dependencies rust-lang/compiler-team#266

Closed

Explain that you have to specify path and repository during subrepo s…

af55352

…ynchronizations

flip1995 reviewed Apr 1, 2020

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

RalfJung reviewed Apr 1, 2020

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

RalfJung reviewed Apr 1, 2020

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

Triple backticks

59cfb80

flip1995 reviewed Apr 2, 2020

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Address review comments

df91b40

Centril reviewed Apr 2, 2020

View reviewed changes

rust-highfive assigned Mark-Simulacrum and unassigned Centril Apr 2, 2020

Address reviews

7928c45

Mark-Simulacrum reviewed Apr 4, 2020

View reviewed changes

Mark-Simulacrum added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 11, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Apr 13, 2020

Dylan-DPC-zz mentioned this pull request Apr 13, 2020

Rollup of 4 pull requests #71111

Merged

bors merged commit 048c842 into rust-lang:master Apr 14, 2020

bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2020

Auto merge of rust-lang#70655 - oli-obk:subrepo_funness, r=Mark-Simul…

53d3bc0

…acrum Make clippy a git subtree instead of a git submodule r? @eddyb cc rust-lang#70651 documentation at rust-lang#70654

matthiaskrgr reviewed May 4, 2020

View reviewed changes

oli-obk deleted the subrepo_docs branch May 4, 2020 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explain how to work with subtree #70654

Explain how to work with subtree #70654

oli-obk commented Apr 1, 2020

Centril commented Apr 1, 2020

oli-obk commented Apr 1, 2020

nikomatsakis commented Apr 1, 2020

RalfJung commented Apr 1, 2020 •

edited

Loading

Centril Apr 2, 2020

eddyb Apr 2, 2020

Centril commented Apr 2, 2020

nikomatsakis commented Apr 3, 2020 •

edited

Loading

Mark-Simulacrum Apr 4, 2020

oli-obk Apr 6, 2020

RalfJung Apr 6, 2020

eddyb Apr 6, 2020

flip1995 Apr 6, 2020

Mark-Simulacrum Apr 4, 2020

Centril Apr 4, 2020

oli-obk Apr 6, 2020

Mark-Simulacrum commented Apr 4, 2020 •

edited

Loading

flip1995 commented Apr 5, 2020

Mark-Simulacrum commented Apr 5, 2020

flip1995 commented Apr 5, 2020

RalfJung commented Apr 5, 2020

nikomatsakis commented Apr 6, 2020

Mark-Simulacrum commented Apr 11, 2020

Mark-Simulacrum commented Apr 13, 2020

bors commented Apr 13, 2020

matthiaskrgr May 4, 2020

oli-obk May 4, 2020

Explain how to work with subtree #70654

Explain how to work with subtree #70654

Conversation

oli-obk commented Apr 1, 2020

Centril commented Apr 1, 2020

oli-obk commented Apr 1, 2020

nikomatsakis commented Apr 1, 2020

RalfJung commented Apr 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril commented Apr 2, 2020

nikomatsakis commented Apr 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mark-Simulacrum commented Apr 4, 2020 • edited Loading

flip1995 commented Apr 5, 2020

Mark-Simulacrum commented Apr 5, 2020

flip1995 commented Apr 5, 2020

RalfJung commented Apr 5, 2020

nikomatsakis commented Apr 6, 2020

Mark-Simulacrum commented Apr 11, 2020

Mark-Simulacrum commented Apr 13, 2020

bors commented Apr 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Apr 1, 2020 •

edited

Loading

nikomatsakis commented Apr 3, 2020 •

edited

Loading

Mark-Simulacrum commented Apr 4, 2020 •

edited

Loading